Move exception handling logic to Route#2026
Conversation
|
I'm positive towards this PR. Is any other member that wants to check it? @encode/maintainers |
|
@tomchristie it would be great to get your eyes on this since no one has really changed this part of the codebase since you originally wrote it |
Kludex
left a comment
Kludex
left a comment
There was a problem hiding this comment.
Are you strong about this code structure?
Do you mind if we maintain the exceptions.py and create a _<some_good_name_at_root_level>? I feel like importing wrap_app_handling_exceptions from middleware/exceptions is not very logic. 🤔
@encode/maintainers If someone is against this idea, or has any concerns regarding this, or want more context, anything... Please let us know. Otherwise, I'll get this merged end of next week.
#1692 also resolves it in a way that compliments this PR, and without breaking other things. |
|
Folks, it would be great to get some more input/review here. |
|
The last bits here: What's our plan regarding the Also, should we update the documentation: https://www.starlette.io/exceptions/#errors-and-handled-exceptions ? |
Kludex
left a comment
Kludex
left a comment
There was a problem hiding this comment.
Do you mind if we change the structure of the files around (I can change it)?
I'd prefer to not remove the file exceptions.py, and just create a top level _exception_handler.py.
|
Sure go for it |
|
@adriangb I didn't change the documentation, but I think we should do something there for 1.0. Also, I applied this commit: |
Co-authored-by: Marcelo Trylesinski <marcelotryle@gmail.com>
3 participants
Fixes #493, partially resolves #1692 (comment), resolves #1649 (comment)
By moving handling of exceptions to the
Routelevel we can use the sameRequest/WebSocketobject for the endpoint and exception handlers.See #2020 for some more background